fix(hub): orphaned agents on a dead host no longer block its deletion - #493
Merged
Conversation
Deleting an offline host 409'd with "host still has active agents" even though its host-runner was gone and the agents had died with it. Two separate defects met in that one refusal. First, handleDeleteHost spelled the terminal-status set differently from every other live-agent query in the package: it listed terminated and failed but not crashed. Since 'crashed' is exactly what the host-runner's reconcile loop reports for an agent whose pane has vanished, the hub's own verdict that an agent was dead was not enough to let its host go. Hoist the predicate into one definition (agent_status.go) and use it at all five sites, so the two spellings cannot drift apart again. Second, even with the set corrected, an agent that never reached a terminal status was a dead end. The hub only reaches an agent through its host-runner; when the runner is gone nothing will ever report that agent terminal, so the refusal could not be satisfied by waiting. The fleet view showed agents "running" on a machine that no longer existed and the row could only be cleared by stopping each agent by hand. Now the host's own status decides: online with live agents still 409s (the runner is there and can stop them properly), offline reaps them to 'crashed' and proceeds. Reaping deliberately does NOT run on a timer. An offline host is not proof of a dead agent — M4 agents live in tmux panes that outlive a host-runner restart, so an agent can be healthy while its runner is upgrading. A clock would kill those irreversibly, since tickReconcile skips terminal agents and a wrong 'crashed' never heals back. The director's DELETE is the judgement that the machine is gone; that is where the reap belongs. The reap runs the same aftermath as any crash — session paused, bearer revoked, digest sealed, task derived — via applyAgentCrashEffects, extracted from handlePatchAgent so a dead agent's token cannot outlive it through the new path. Covers the handler, which had no tests: terminal agents never block, online still refuses, offline reaps, the aftermath runs, the reap is scoped to the host, and the SQL agrees with the Go predicate. Each verified by mutation — reintroducing the missing 'crashed' reproduces the original 409. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| for _, ag := range live { | ||
| if err := s.reapOrphanedAgent(r.Context(), team, ag.ID); err != nil { | ||
| s.log.Warn("reap orphaned agent failed", | ||
| "agent", ag.ID, "host", host, "err", err) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
Deleting an offline host 409s with
host still has active agents — terminate them first, even though the host-runner is gone and the agents died with it. Two defects meet in that one refusal.1. One query disagreed with the rest of the package about what "alive" means
handleDeleteHostcounted live agents withstatus NOT IN ('terminated','failed'). Every other live-agent query in the package uses('terminated','crashed','failed')— and so does migration 0024's handle-uniqueness index.crashedis precisely what the host-runner's reconcile loop reports for an agent whose pane has vanished (hostrunner/reconcile.go:126). So the hub's own verdict that an agent was dead was not enough to let its host go.Hoisted into one definition (
agent_status.go) and used at all five sites, so the spellings cannot drift apart again.2. A live agent on a dead host was a dead end
Even with the set corrected, an agent that never reached a terminal status blocked forever. The hub only reaches an agent through its host-runner; with the runner gone, nothing will ever report that agent terminal, so the refusal could not be satisfied by waiting. The fleet view showed agents "running" on a machine that no longer existed, and the only way out was stopping each agent by hand.
Now the host's own status decides:
onlineofflinecrashed, delete proceedsWhy not a background reaper
An offline host is not proof of a dead agent. M4 agents launch into tmux windows (
TmuxLauncher), and the tmux server outlives a host-runner restart — an agent can be perfectly healthy while its runner is being upgraded. Only M1/M2 agents are children of the runner (exec.CommandContext+cmd.Start()inlaunch_m2.go) and die with it.A reaper on a clock would kill the healthy ones, and irreversibly:
tickReconcileskips agents already in a terminal status, so a wrongcrashedverdict never heals back torunning— and the driver-teardown pass then stops its driver for real.The director's DELETE is the judgement that the machine is gone. That is where the reap belongs.
Reaping is not just a status write
reapOrphanedAgentruns the same aftermath as any crash — session paused, bearer token revoked, run digest sealed, task status derived (ADR-029 D-3 readscrashed→blocked, distinct from a terminate-with-no-result →cancelled). That aftermath was inline inhandlePatchAgent; extracted toapplyAgentCrashEffectsso a dead agent's token cannot outlive it through the new path.Tests
The handler had none. Added: terminal agents never block (all three statuses), online still refuses and touches nothing, offline reaps, the aftermath runs, the reap is scoped to its own host, missing host is 404, and the SQL predicate agrees with the Go one.
Each verified by mutation:
crashed→TestDeleteHost_TerminalAgentsNeverBlock/crashedfails with the exact 409 from the reportactivego test ./...green; full lint set green (openapi validated in a venv).Not covered
No live-fleet reproduction — this was traced and fixed from the code and exercised against the real handler through the router in tests. Worth one manual delete of a genuinely dead host after deploy. Note the hub takes up to ~120s (
HostStaleThreshold+ sweep interval) to flip a host tooffline; a delete attempted inside that window still 409s by design.🤖 Generated with Claude Code